👷 change e2e implementation for workers #3901
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Bundles Sizes Evolution
🚀 CPU Performance
🧠 Memory Performance
|
|
✅ Tests 🎉 All green!❄️ No new flaky tests detected 🎯 Code Coverage 🔗 Commit SHA: 1ea0c00 | Docs | Datadog PR Page | Was this helpful? Give us feedback! |
mormubis
changed the title
mormubis
force-pushed
the
adlrb/logs-improve-testing
branch
from
0feb678 to
a5b8ded
Compare
bcaudan
reviewed
Oct 13, 2025
mormubis
force-pushed
the
adlrb/logs-improve-testing
branch
from
9afa674 to
3be6403
Compare
bcaudan
reviewed
Oct 17, 2025
mormubis
force-pushed
the
adlrb/logs-improve-testing
branch
from
3d6c391 to
4a69138
Compare
bcaudan
reviewed
Oct 23, 2025
test/e2e/lib/framework/pageSetups.ts
Outdated
| export function workerSetup(options: WorkerOptions, servers: Servers) { | ||
| return js` | ||
| ${options.importScripts ? js`importScripts('/datadog-logs.js');` : js`import '/datadog-logs.js';`} | ||
| export function workerSetup(options: SetupOptions, servers: Servers) { |
There was a problem hiding this comment.
💭 thought: as this helper is not really related to the page setup, maybe we could move it to createTest to colocate it with other worker setup code and avoid the back and forth between those files.
test/e2e/lib/framework/createTest.ts
Outdated
| navigator.serviceWorker.register('/sw.js', ${JSON.stringify(options)}).then((registration) => { | ||
| window.myServiceWorker = registration | ||
| }) |
There was a problem hiding this comment.
💬 suggestion: if we don't plan to use interactWithWorker in the coming tests, we could remove it and the window.myServiceWorker reference.
There was a problem hiding this comment.
Removed, I kept the window.myServiceWorker to avoid double registering.
mormubis
force-pushed
the
adlrb/logs-improve-testing
branch
from
4a69138 to
762681a
Compare
bcaudan
reviewed
Oct 24, 2025
test/e2e/lib/framework/createTest.ts
Outdated
| servers.base.bindServerApp( | ||
| createMockServerApp(servers, setup, { | ||
| remoteConfiguration: setupOptions.remoteConfiguration, | ||
| workerImplementation: setupOptions.workerImplementationFactory && workerSetup(setupOptions, servers), |
There was a problem hiding this comment.
💬 suggestion: the check is also done inside the function, so we could probably remove it from here
Suggested change
| workerImplementation: setupOptions.workerImplementationFactory && workerSetup(setupOptions, servers), | |
| workerImplementation: workerSetup(setupOptions, servers), |
mormubis
force-pushed
the
adlrb/logs-improve-testing
branch
from
10587ae to
1ea0c00
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
The initial implementation of Worker E2E tests was somewhat tied to our specific use case and will not scale if we need to test different scenarios within the Worker environment.
Changes
It introduces a version of
withWorkerthat automatically sets up the logs inside the worker. We can also pass an implementation, similar to what we do withwithLogsandwithBody.Test instructions
All e2e still passed.
Checklist